Logical dns: moving to an extension#23799
Conversation
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk
force-pushed
the
logical_dns
branch
from
7ed2e16 to
cf655b5
Compare
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
Thanks, left a few questions/comments.
| envoy.clusters.logical_dns: | ||
| categories: | ||
| - envoy.clusters | ||
| security_posture: requires_trusted_downstream_and_upstream |
There was a problem hiding this comment.
Is this the right posture? I was under the impression that as part of core clusters it was "robust_to_untrusted_downstream_and_upstream", but maybe I'm wrong.
There was a problem hiding this comment.
good catch thanks, fixed.
| "//source/common/api:api_lib", | ||
| "//source/exe:envoy_main_common_with_core_extensions_lib", | ||
| "//source/exe:platform_impl_lib", | ||
| "//source/extensions/clusters/logical_dns:logical_dns_cluster_lib", |
There was a problem hiding this comment.
Hmm... I wonder if this target can be built w/o reliance on source/extensions.
There was a problem hiding this comment.
no, there's already a number of registered extensions such as source/extensions/transport_sockets/raw_buffer/config.h that envoy won't build without
There was a problem hiding this comment.
that said this is why we have the visibility rules and an extra presubmit around extensions not generally being visible to core tests. so it's a pretty well cross-checked system :-)
| ":http_protocol_integration_lib", | ||
| ":socket_interface_swap_lib", | ||
| "//source/common/http:header_map_lib", | ||
| "//source/extensions/clusters/logical_dns:logical_dns_cluster_lib", |
There was a problem hiding this comment.
Similar question to the //test/exe:main_common_test target.
I guess it is ok because these are tests, but wonder if the extension is inherent to the tests functionality or is it testing non-core functionality (and then the test itself may be moved to the be part of the extension's tests)?
|
/retest |
|
Retrying Azure Pipelines: |
3 participants
Any Envoy users who customize their pre-built extensions will need to evaluate if they need this cluster.
Akin to #23694
Risk Level: medium
Testing: n/a
Docs Changes: n/a
Release Notes: inline